Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spring-webflux 5.3 instrumentation #121

Merged
merged 2 commits into from
Dec 15, 2020
Merged

Spring-webflux 5.3 instrumentation #121

merged 2 commits into from
Dec 15, 2020

Conversation

tspring
Copy link
Contributor

@tspring tspring commented Nov 19, 2020

Fixes #104

@hisener
Copy link

hisener commented Dec 7, 2020

@tspring @jodeev is there any timeline for WebFlux 5.3.0+ support? Let me know if I can help in any way.

@tspring
Copy link
Contributor Author

tspring commented Dec 8, 2020

Hi @hisener , While this PR hasn't been reviewed yet, I would expect it to be in the next week or two and included in the next agent release (6.3.0). If you'd be willing to test these changes and offer any feedback, that'd be welcome as well! Cheers.

Copy link
Contributor

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tspring can you summarize the changes from 5.1.0 => 5.3.0 and how the instrumentation has been adjusted to reflect the changes?

if (token != null) {
token.linkAndExpire();
}
runnable.run();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed that this line is missing in the same file in spring-webflux-5.1.0 and spring-webflux-5.0.0 modules. Is that a bug there or here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was the issue fixed in #114

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah I see so this branch just hasn't been rebased to in include that code.


@RunWith(InstrumentationTestRunner.class)
@InstrumentationTestConfig(includePrefixes = { "org.springframework" })
public class Spring510RouterFunTest {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename to Spring530RouterFunTest, no?

@tspring
Copy link
Contributor Author

tspring commented Dec 8, 2020

@jack-berg the field here was changed to no longer be final. That's the only difference between the 5.1 instrumentation and 5.3

WeaveViolation{type=FIELD_FINAL_MISMATCH, clazz=org/springframework/web/reactive/function/server/RequestPredicates$PathPatternPredicate, field=pattern}

@tspring tspring merged commit efb4a4c into main Dec 15, 2020
@tspring tspring deleted the spring-webflux-5.3 branch December 15, 2020 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Add instrumentation for spring-webflux 5.3.0+
3 participants